Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove dependency on Dispatchers.Main.immediate for immediate dispatching in Compose. #2725

Open
wants to merge 30 commits into
base: main
Choose a base branch
from

Conversation

colinrtwhite
Copy link
Member

@colinrtwhite colinrtwhite commented Nov 29, 2024

Fixes an issue with Coil's public composables where its memory cache check could miss the first frame due to dispatching. This case occurs with Paparazzi/Roborazzi/AndroidX screenshot tests and is due to the dispatcher provided by rememberCoroutineScope() either not supporting immediate dispatching, Dispatchers.Main.immediate being unavailable, or Dispatchers.Main.immediate using a different underlying thread.

To fix this I tried a couple things:

  • Using CoroutineStart.UNDISPATCHED with a StateFlow. This works for initial composition, but does not resolve synchronously from the memory cache on subsequent requests (e.g. when the AsyncImage(model) changes).
  • Attempting to create a CoroutineDispatcher that re-enables dispatching after the first time the context's CoroutineDispatcher changes. As far as I can tell this isn't possible at the moment with CoroutineInterceptor's hooks.

I settled on using a custom CoroutineDispatcher that acts like Dispatchers.Unconfined until immediately after the memory cache check. This is safe for Coil's internals as it's all thread safe. Aside from fixing the above issue this has a number of benefits:

  • This ensures we have consistent behaviour in all environments instead of falling back to Dispatchers.Unconfined if Dispatchers.Main.immediate is unavailable.
  • kotlinx-coroutines-swing no longer needs to be added manually as a dependency on JVM.

@colinrtwhite colinrtwhite force-pushed the colin/delayed_dispatch branch from cb33907 to cd47369 Compare December 3, 2024 17:26
@colinrtwhite colinrtwhite force-pushed the colin/delayed_dispatch branch 2 times, most recently from 083e410 to ea0bbbc Compare December 20, 2024 01:52
@colinrtwhite colinrtwhite force-pushed the colin/delayed_dispatch branch from e2cda61 to 3fe0ca5 Compare December 22, 2024 20:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant